Additional Unit Tests for Parquet VARIANT Field Extraction - #23036

I might be missing context. I don't really know what this means. Is this 0 a value in the Parquet specification that represents null? Is this representing a type id, or a value?

Is uint8_t a type declared in the Parquet variant spec?

Hi @bdice these tests are targeting casting from sample raw variant bytes (like the val vector here and other tests). We extract these bytes out from variant rows as col<list<uint8_t>> (one list per extracted field per variant row)

Variant looks like this:

First byte -> header

bits 0-1: basic_type   (0=primitive, 1=short_string, 2=object, 3=array)
bits 2-7: value_header (6 bits, meaning depends on basic_type)

For basic_type::primitive (0), the 6-bit value_header is a physical type id (0=null, 1/2=bool true/false, 3-6=int8/16/32/64, 7=float64, 16=long_string, etc. from the enum). 
For basic_type::short_string (1), the 6-bit value_header is the string length (0-63 bytes) and the string bytes follow immediately.

In this test, there's just one byte 0x0 saying primitive type but no other info on what type and what data (so it throws). Please correct me if I am wrong @abigalekim

Similarly in the next test, we have 0x1 (indicating short string) but nothing really after so it should decode to a valid but empty ("") value.

Here's a summary of what's going on in each test that I got from AI:

UnsupportedCastTypeThrows — uses val = {0x00}. Byte 0x00 = basic_type 00 (primitive) + value_header 000000 (primitive_type::null), i.e. a Variant "null" value. This is fine as input, but it's actually a bit of a red herring: cast_variant's type-check happens entirely at compile-time dispatch (cast_variant_fn::operator() with requires(not is_variant_castable<T>) → CUDF_FAIL), based purely on the desired output type (FLOAT64/BOOL8), not on the byte content. Any well-formed list<uint8> row would trigger the same throw — this matches exactly what bdice asked about in review ("Is this 0 a value...type id, or a value?").

ShortStringLengthZero — val = {0x01}. 0x01 = basic_type=01 (short_string), value_header = 0 (length 0). Per decode_string, with btype == short_string, str_len = value_header = 0, so it slices 0 bytes after the header → empty string. This exercises the lower boundary of the 6-bit length field.

ShortStringLengthSixtyThree — header 0xFD = binary 11111101: bits 0-1 = 01 (short_string), bits 2-7 = 111111 = 63 (the max value a 6-bit field can hold, i.e. the largest string that can use the short-string encoding before Parquet must switch to long_string). Followed by 63 'z' payload bytes. This is the correctness boundary check corresponding to 0x01 | (63 << 2) = 0xFD in the test comment.

LongStringLengthZero — val = {0x40, 0x00, 0x00, 0x00, 0x00}. 0x40 = binary 01000000: bits 0-1 = 00 (primitive), bits 2-7 = 010000 = 16 = primitive_type::long_string. Per the spec, a long_string primitive is followed by a 4-byte little-endian length, then that many payload bytes — here the length is 0x00000000 = 0, so no payload follows.

LongStringTruncatedPayloadYieldsNull — val = {0x40, 0x0A, 0x00, 0x00, 0x00, 'a','b','c'}: same long_string header, but the LE length field claims 0x0000000A = 10 bytes of payload while only 3 ("abc") are actually present. This targets the truncation guard in decode_string:

@abigalekim can you please double check this and perhaps add some info from here to the tests so it's easy to review 😄. Perhaps we can add a comment/link to the schema so we can refer to it while reviewing.

Should we have something like a factory that builds a byte from named enums?

Like replace 0x06 with make_variant_primitive(variant_type_id::INT64)?

Then it’s self-explanatory at the call site.

Also having a factory would help resolve any ambiguity between big-endian and little-endian encodings. (Are bits 0-1 low or high?)

I think this is a great idea to have a reusable factory + provides context for (current and future) variant tests.

Hi, I added small comments describing what each test is, and implemented a reusable factory for writing tests! I think the AI descriptions seem correct.

Where do the values 0x01 | (0 << 2) come from?

This test now uses the factory, but it was the manual construction of the VARIANT short-string value-header byte.

There appears to be a real gap in testing (and likely requiring an implementation change) where cast_variant bypasses the validation for empty inputs.

The implementation returns make_empty_column(desired_type) before it reaches the unsupported-type dispatcher, so cast_variant(empty_list_uint8, FLOAT64) can succeed even though unsupported desired_type values are documented to throw.

I think this should be its own PR. @vuule thoughts?

@vuule is OOTO so let's not ping him here 🙂. @abigalekim if you would like to handle this in a separate PR, please open an issue (perhaps with a proposed test/repro) against this comment so it could be tracked

missed this until now. Yeah, separate PR sounds good. Keep them focused so they move faster.

@abigalekim Please create an issue for this so we don't loose track.

I made an issue for this problem #23361!